Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Sqlite migration and foreignKey support #612

Closed
wants to merge 2 commits into from

Conversation

marcj
Copy link
Member

@marcj marcj commented Feb 18, 2013

Just need the PR id in another ticket. Maybe it's done in a few days.

MArcJ added 2 commits February 18, 2013 01:48
…sqlite' in the 'bookstore' database section. (for testing purposes)

This is in WIP. Next step is that travis-ci uses several environments where we test sqlite/pgsql/mysql, not as currently where the most tests are against mysql.
This generated a couple of issues since some tests got the ($this->con) connection from the wrong database. Also fixed a bunch of errors with the new SQLite stuff.
* @param ForeignKey $fk
*/
public function removeForeignKey(ForeignKey $fk){
$idx = array_search($fk, $this->foreignKeys);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use isset instead because its a lot faster

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's about removing the FK, not checking.

@staabm
Copy link
Member

staabm commented Feb 18, 2013

sorry for the early feedback, I just recognized its an WIP...

@ssokolow
Copy link

ssokolow commented May 4, 2013

Has there been any progress on this? I have a project which I'd like to move to a fatter model but whether I wait and switch to Propel or just jerry rig something on top of my existing solution will depend on how likely this is to be ready within the next few months.

@staabm
Copy link
Member

staabm commented May 4, 2013

You are welcome to provide the remaing changes for this PR. I don't know if @marcj will have time for this

@ssokolow
Copy link

ssokolow commented May 4, 2013

Given how little time I have, the "within the next few months" may be a limit imposed by when I have time to migrate to Propel or even just make changes to my existing database code.

As much as I'd love to help, there's no way I have time to grow familiar enough with the Propel codebase to contribute a patch in the near future.

However, in the longer term (say... July, maybe?), it's a possibility if you or someone else familiar with the project can point me to a crash course in how the Propel codebase is laid out, how comprehensive the unit tests are, what standards need to be met for this patch to get accepted (including what extra unit tests I might have to write), etc.

@staabm
Copy link
Member

staabm commented May 4, 2013

Sure. Just give us a ping when you have time

@marcj
Copy link
Member Author

marcj commented May 27, 2013

I'll close this PR soon and will integrate it in Propel2.

@ssokolow
Copy link

@marcj

In three days, it'll have been two months since you said "soon". I'd like to start using Propel2 on an SQLite-backed project before my vacation ends.

Did you forget or is there some hold-up I might be able to handle for you?

@marcj
Copy link
Member Author

marcj commented Jul 25, 2013

Unfortunately, I'm not able to work on this issue, since I'm working on another big project for the next weeks. But I guess I need this PR as well in that project (but in Propel2). However, this PR is 'nearly' done, so it shouldn't be much work to complete it.

@marcj
Copy link
Member Author

marcj commented Aug 20, 2013

I've implemented those features in Propel2: propelorm/Propel2#433

@marcj marcj closed this Aug 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants